Add request filter for early user and query parsing by vishalya · Pull Request #748 · trinodb/trino-gateway

vishalya · 2025-08-11T20:03:14Z

Description

The parsing of the user and query was happening at different places and at a later stages, this PR structures it to happen at a early stage and a single point in time.

Additional context and related issues

Early Request Parsing: New JAX-RS filters have been added to intercept incoming requests. This filter is responsible for parsing essential user and query information at the earliest stage of processing under the PRE_AUTHENTICATION and PRE_AUTHORIZATION.
Use of ContainerRequestContext: The implementation now uses ContainerRequestContext instead of HttpServletRequest to access request details, as this is the object available within the JAX-RS filter context.
Injecting Request Attributes: The extracted user and query information is now passed along via request attributes, making it easily accessible in the later stages of request routing

Release notes

(X) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required, with the following suggested text:

vishalya · 2025-08-12T16:50:50Z

Addressed the review comments.

oneonestar · 2025-09-02T07:54:14Z

How about using a single filter to parse all the data instead of having multiple filters?
We can store all the necessary data in a single object.
I think this would simplify the code and also help avoid dependency or execution order issues between multiple filters in the future.

vishalya · 2025-09-03T13:23:08Z

I would like to keep it separate as they are different and will be parsed at different priorities (user info might need to be parsed at pre-authentication and query might be parsed at pre-authorization), The query information may not be present for all the requests.

vishalya · 2025-09-16T21:03:14Z

Added a new class PathFilter to reuse the logic in the filters.

vishalya · 2025-09-16T21:03:28Z

@ebyhr - I have addressed the review comments.

xkrogen · 2025-10-17T17:12:39Z

@vishalya are you still working on this? Would love to see it land -- this is a great cleanup

vishalya · 2025-10-27T18:42:12Z

I’ll spend some time on the code comments this week

Chaho12

LGTM. Can you resolve comments if they are addressed :)

Chaho12 · 2025-11-03T23:11:24Z

Let's get this merged. This refactoring would affect others quite a bit.

ebyhr · 2025-11-04T04:48:35Z

+    public PathFilter(
+            List<String> statementPaths,
+            List<String> extraWhitelistPaths)


Please inject HaGatewayConfiguration instead.

ebyhr · 2025-11-04T04:49:11Z

+            List<String> statementPaths,
+            List<String> extraWhitelistPaths)
+    {
+        this.statementPaths = Set.copyOf(requireNonNull(statementPaths, "Required configuration 'statementPaths' can't be null"));


Use ImmutableSet.copyOf and remove requireNonNull.

ebyhr · 2025-11-04T04:50:41Z

+        this("", "", "", ImmutableList.of(), Optional.empty(), Optional.empty(),
+                ImmutableSet.of(), ImmutableSet.of(), ImmutableSet.of(), false, Optional.empty());


The convention is single line or separate lines.

this( "", "", "", ImmutableList.of(), Optional.empty(), Optional.empty(), ImmutableSet.of(), ImmutableSet.of(), ImmutableSet.of(), false, Optional.empty());

ebyhr · 2025-11-04T04:52:21Z

+    private Optional<String> extractUserFromCookies(ContainerRequestContext requestContext, String userField)
    {
-        if (request.getCookies() == null) {
+        Map<String, jakarta.ws.rs.core.Cookie> cookies = requestContext.getCookies();


Import jakarta.ws.rs.core.Cookie.

ebyhr · 2025-11-04T04:53:20Z

+    private static final int MAX_QUERY_TEXT_LOG_LENGTH = 100;
+    private final boolean isAnalyzeRequest;


Add an empty line between constants and fields.

ebyhr · 2025-11-04T04:59:09Z

+        try {
+            when(uriInfo.getRequestUri()).thenReturn(new URI("http://localhost" + HttpUtils.OAUTH_PATH));
+        }
+        catch (URISyntaxException e) {
+            throw new RuntimeException(e);
+        }


Remove redundant try-catch.

ebyhr · 2025-11-04T04:59:14Z

+        try {
+            when(uriInfo.getRequestUri()).thenReturn(new URI("http://localhost" + HttpUtils.V1_STATEMENT_PATH));
+        }
+        catch (URISyntaxException e) {
+            throw new RuntimeException(e);
+        }


Remove redundant try-catch.

ebyhr · 2025-11-04T04:59:20Z

+        try {
+            when(uriInfo.getRequestUri()).thenReturn(new URI("http://localhost" + HttpUtils.V1_STATEMENT_PATH));
+        }
+        catch (URISyntaxException e) {
+            throw new RuntimeException(e);
+        }


Remove redundant try-catch.

ebyhr · 2025-11-04T04:59:28Z

+        try {
+            when(uriInfo.getRequestUri()).thenReturn(new URI("http://localhost" + HttpUtils.V1_STATEMENT_PATH));
+        }
+        catch (URISyntaxException e) {
+            throw new RuntimeException(e);
+        }


Remove redundant try-catch.

ebyhr · 2025-11-04T04:59:54Z

+
+final class TestQueryUserInfoParser
+{
+    private QueryUserInfoParser filter;


Make this field constant.

cla-bot Bot added the cla-signed label Aug 11, 2025

vishalya requested review from Chaho12, RoeyoOgen, andythsu and oneonestar August 11, 2025 20:03

Chaho12 reviewed Aug 12, 2025

View reviewed changes

vishalya force-pushed the parse-user-query branch from 5c57364 to 28caef2 Compare August 12, 2025 16:50

RoeyoOgen reviewed Aug 17, 2025

View reviewed changes

ebyhr requested changes Aug 26, 2025

View reviewed changes

vishalya force-pushed the parse-user-query branch 2 times, most recently from 239f66a to 68e1860 Compare August 29, 2025 20:45

vishalya force-pushed the parse-user-query branch 2 times, most recently from b97df7f to b202da0 Compare September 16, 2025 20:36

Chaho12 reviewed Sep 17, 2025

View reviewed changes

Comment thread gateway-ha/src/main/java/io/trino/gateway/ha/router/TrinoQueryProperties.java

Comment thread gateway-ha/src/main/java/io/trino/gateway/ha/router/TrinoQueryProperties.java Outdated

Chaho12 reviewed Sep 17, 2025

View reviewed changes

vishalya force-pushed the parse-user-query branch 2 times, most recently from 910492f to 1d71ebd Compare September 26, 2025 20:02

vishalya force-pushed the parse-user-query branch from 1d71ebd to 48a3b6f Compare October 2, 2025 15:42

vishalya force-pushed the parse-user-query branch from 48a3b6f to e20aff9 Compare October 28, 2025 18:55

Chaho12 approved these changes Oct 29, 2025

View reviewed changes

vishalya force-pushed the parse-user-query branch from 9ea3587 to bf4cdbf Compare October 31, 2025 20:17

Add request filter for early user and query parsing

a75b9fe

vishalya force-pushed the parse-user-query branch from bf4cdbf to a75b9fe Compare October 31, 2025 20:56

Chaho12 merged commit 5a36940 into trinodb:main Nov 3, 2025
3 checks passed

github-actions Bot added this to the 17 milestone Nov 3, 2025

ebyhr reviewed Nov 4, 2025

View reviewed changes

mosabua mentioned this pull request Nov 7, 2025

Add Trino Gateway 17 release notes #792

Merged

		this("", "", "", ImmutableList.of(), Optional.empty(), Optional.empty(),
		ImmutableSet.of(), ImmutableSet.of(), ImmutableSet.of(), false, Optional.empty());

		private static final int MAX_QUERY_TEXT_LOG_LENGTH = 100;
		private final boolean isAnalyzeRequest;

Conversation

vishalya commented Aug 11, 2025

Description

Additional context and related issues

Release notes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vishalya commented Aug 12, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

oneonestar commented Sep 2, 2025

Uh oh!

vishalya commented Sep 3, 2025

Uh oh!

vishalya commented Sep 16, 2025

Uh oh!

vishalya commented Sep 16, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

xkrogen commented Oct 17, 2025

Uh oh!

vishalya commented Oct 27, 2025

Uh oh!

Chaho12 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Chaho12 commented Nov 3, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!